Extract path from AppConfiguration into App.configPath#6949
Conversation
Coverage report
Test suite run success3806 tests passing in 1462 suites. Report generated by 🧪jest coverage report action from d965024 |
ae09959 to
6a0f47a
Compare
405c900 to
74e0e58
Compare
3813e7f to
530df59
Compare
74e0e58 to
6e51da6
Compare
530df59 to
fcff02d
Compare
c1e50a8 to
8ae41eb
Compare
fcff02d to
5028d72
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationspackages/cli-kit/dist/public/node/toml/codec.d.tsimport { JsonMap } from '../../../private/common/json.js';
export type JsonMapType = JsonMap;
/**
* Given a TOML string, it returns a JSON object.
*
* @param input - TOML string.
* @returns JSON object.
*/
export declare function decodeToml(input: string): JsonMapType;
/**
* Given a JSON object, it returns a TOML string.
*
* @param content - JSON object.
* @returns TOML string.
*/
export declare function encodeToml(content: JsonMap | object): string;
packages/cli-kit/dist/public/node/toml/index.d.tsexport type { JsonMapType } from './codec.js';
packages/cli-kit/dist/public/node/toml/toml-file.d.tsimport { JsonMapType } from './codec.js';
/**
* Thrown when a TOML file cannot be parsed. Includes the file path for context.
*/
export declare class TomlParseError extends Error {
readonly path: string;
constructor(path: string, cause: Error);
}
/**
* General-purpose TOML file abstraction.
*
* Provides a unified interface for reading, patching, removing keys from, and replacing
* the content of TOML files on disk.
*
* - `read` populates content from disk
* - `patch` does surgical WASM-based edits (preserves comments and formatting)
* - `remove` deletes a key by dotted path (preserves comments and formatting)
* - `replace` does a full re-serialization (comments and formatting are NOT preserved).
* - `transformRaw` applies a function to the raw TOML string on disk.
*/
export declare class TomlFile {
/**
* Read and parse a TOML file from disk. Throws if the file doesn't exist or contains invalid TOML.
* Parse errors are wrapped in {@link TomlParseError} with the file path for context.
*
* @param path - Absolute path to the TOML file.
* @returns A TomlFile instance with parsed content.
*/
static read(path: string): Promise<TomlFile>;
readonly path: string;
content: JsonMapType;
constructor(path: string, content: JsonMapType);
/**
* Surgically patch values in the TOML file, preserving comments and formatting.
*
* Accepts a nested object whose leaf values are set in the TOML. Intermediate tables are
* created automatically. Setting a leaf to `undefined` removes it (use `remove()` for a
* clearer API when deleting keys).
*
* @example
* ```ts
* await file.patch({build: {dev_store_url: 'my-store.myshopify.com'}})
* await file.patch({application_url: 'https://example.com', auth: {redirect_urls: ['...']}})
* ```
*/
patch(changes: {
[key: string]: unknown;
}): Promise<void>;
/**
* Remove a key from the TOML file by dotted path, preserving comments and formatting.
*
* @param keyPath - Dotted key path to remove (e.g. 'build.include_config_on_deploy').
* @example
* ```ts
* await file.remove('build.include_config_on_deploy')
* ```
*/
remove(keyPath: string): Promise<void>;
/**
* Replace the entire file content. The file is fully re-serialized — comments and formatting
* are NOT preserved.
*
* @param content - The new content to write.
* @example
* ```ts
* await file.replace({client_id: 'abc', name: 'My App'})
* ```
*/
replace(content: JsonMapType): Promise<void>;
/**
* Transform the raw TOML string on disk. Reads the file, applies the transform function
* to the raw text, writes back, and re-parses to keep `content` in sync.
*
* Use this for text-level operations that can't be expressed as structured edits —
* e.g. Injecting comments or positional insertion of keys in arrays-of-tables.
* Subsequent `patch()` calls will preserve any comments added this way.
*
* @param transform - A function that receives the raw TOML string and returns the modified string.
* @example
* ```ts
* await file.transformRaw((raw) => `# Header comment\n${raw}`)
* ```
*/
transformRaw(transform: (raw: string) => string): Promise<void>;
private decode;
}
Existing type declarationsWe found no diffs with existing type declarations |
8ae41eb to
a29ba87
Compare
5028d72 to
49d9789
Compare
a29ba87 to
dab1a61
Compare
49d9789 to
29d8b4d
Compare
dab1a61 to
2378a7f
Compare
Path is not data — it's metadata about where the config file lives,
not part of what the TOML file contains. This is the first step toward
making AppConfiguration a trustworthy, file-shaped type.
- Add `configPath: string` to App, AppInterface, AppConfigurationInterface
- Remove `& {path: string}` from AppConfiguration, BasicAppConfigurationWithoutModules, LegacyAppConfiguration
- Delete AppConfigurationWithoutPath (replaced by AppConfiguration)
- Simplify SchemaForConfig (no more Omit<..., 'path'>)
- parseConfigurationFile no longer bolts path onto return value
- writeAppConfigurationFile takes explicit configPath parameter
- Type guards pass objects directly (no destructuring to strip path)
- Migrate ~24 callsites from app.configuration.path → app.configPath
- Remove 10 workarounds (delete file.path, blocklist, Omit wrappers)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts:
# packages/app/src/cli/models/app/app.test-data.ts
# packages/app/src/cli/models/app/app.test.ts
# packages/app/src/cli/models/app/app.ts
# packages/app/src/cli/models/app/loader.test.ts
# packages/app/src/cli/models/app/loader.ts
# packages/app/src/cli/services/app-context.test.ts
# packages/app/src/cli/services/app/config/link.test.ts
# packages/app/src/cli/services/app/config/use.test.ts
# packages/app/src/cli/services/app/env/pull.test.ts
# packages/app/src/cli/services/app/env/show.test.ts
# packages/app/src/cli/services/context.ts
# packages/app/src/cli/services/context/identifiers-extensions.test.ts
# packages/app/src/cli/services/dev.ts
# packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts
# packages/app/src/cli/services/dev/app-events/file-watcher.test.ts
# packages/app/src/cli/services/dev/select-app.test.ts
# packages/app/src/cli/services/info.test.ts
# packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts
29d8b4d to
0af7623
Compare
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
0af7623 to
d965024
Compare
| */ | ||
| export function filterNonVersionedAppFields(configuration: object): string[] { | ||
| const builtInFieldNames = Object.keys(AppSchema.shape).concat('path', 'organization_id') | ||
| const builtInFieldNames = Object.keys(AppSchema.shape).concat('organization_id') |
There was a problem hiding this comment.
We should probably check if we can remove this organization_id, there was a period of time where we had it in the toml, but it's not there anymore. It might be for some old apps, but it isn't part of the schema.
| .filter((key) => !extensionInstancesWithKeys.some(([_, keys]) => keys.includes(key))) | ||
| .filter((key) => { | ||
| const configKeysThatAreNeverModules = [...Object.keys(AppSchema.shape), 'path', 'organization_id'] | ||
| const configKeysThatAreNeverModules = [...Object.keys(AppSchema.shape), 'organization_id'] |
There was a problem hiding this comment.
same here, we should just test with an app that has organization_id in the toml. but we can probably remove this custom behavior.
There was a problem hiding this comment.
will follow up on this, thanks for flagging!
|
|
||
| // First write | ||
| await writeAppConfigurationFile({...REALISTIC_CONFIG, path: filePath}, schema) | ||
| await writeAppConfigurationFile(REALISTIC_CONFIG as CurrentAppConfiguration, schema, filePath) |
There was a problem hiding this comment.
isn't it easier to cast on REALISTIC_CONFIG itself rather than casting everytime is used?
isaacroldan
left a comment
There was a problem hiding this comment.
Nice cleanup! just two minor comments

WHY are these changes introduced?
Some light cleanup to make maintenance and coming refactors simpler. Today,
AppConfigurationcarries apathfield that was never part of any TOML schema. It was added byparseConfigurationFileand then worked around in many places:Omit<..., 'path'>on schema types,delete file.pathbefore re-validation, destructuring to strip it in type guards, a blocklist to prevent writing it back to disk.This PR establishes a cleaner separation:
AppConfigurationdescribes only what's in the TOML file. The file path is metadata about the app, not part of its configuration.WHAT is this pull request doing?
Moves
pathfrom the configuration type to the app model:App.configPath: string— path is metadata about the app, not part of its config (same patternExtensionInstanceuses withconfigurationPath)AppConfiguration— removes& {path: string}intersection from all 3 config typesAppConfigurationWithoutPath— deleted (no longer needed), 8 usages replaced withAppConfigurationSchemaForConfig— simplified toZodObjectOf<TConfig>(no moreOmit)parseConfigurationFile— no longer appends{path: filepath}to return valuewriteAppConfigurationFile— takes explicitconfigPathparameterpathapp.configuration.path→app.configPathBehaviorally, this is a no-op.
How to test your changes?
Config pipeline snapshot tests confirm no change in TOML output.
Measuring impact
Checklist